what I have realized is that we download docker images specific to integration tests all the time. I am proposing to cache them via github actions instead.

In order to do that, we are going to track what images we use and their versions in this file. It is used in the pipeline as the key of the cache : docker-images-cache-${{ hashFiles('**/current-images.txt') }}.

If this file changed, we will re-download them; it if stays the same and such a cache is present, it will download from the cache.

I would recommend unit testing this somehow. Possibly you can make a test that dumps the images from k3s ctl and fails if it has something that isn't in here or visa versa? Just a thought, as this is very likely to drift.

another way could be to make this like DockerImages.go, and give it a main function that prints out a list like this. Then, as long as you checked out the source first and installed go, you could run that main and also have no drift.

this is just a helper, really. It is supposed to help with the times we run integration tests (that we plan to reduce in the long run anyway). So even if an image is not downloaded from the cache, it is still going to be pulled in the test. I, personally, would not test this "too much" and a few runs that I had proved that it works exactly as intended.

p.s. I meant DockerImages.java ;) ok you can also solve this by making a comment in the java code that defines the istio versions saying "// please update current-images.txt when you update this"

because at the end of the day, it is about drift making the infrastructure added here serve no value.

if you look closely into Commons.java it does not have any versions anymore. That is why I introduced Images.java - that is the sole entry point that deals with versions of images that we use in integration tests. That is also the reason why deployment manifests don't have versions, I read them from that current-images.txt

neither does Fabric8IstioIT : no version there either. Even better, we do not need pilot and proxyv2 at all, so I dropped those. All I need is your PR to be merged first :)

P.S. I doubt that I want to wait until that PR in test-containers is reviewed and released, so I am inclined to suggest merge/review this one the way it is. When that PR sees day light, I will go for another round of minor refactor.

👌

if cache is not present, issue docker pull and docker save into /tmp/docker/images.

It also replaces the / with - in the docker image, otherwise it can't be saved

we don't provide versions here, but read what these versions are from current-images.txt using a newly introduced class : Images.

this class is the sole entry point when we need to find a certain version of an image, or load a container into kind

this Commons.loadImageFromPath(found.get(), container); will only happen when running inside github actions, because we create a path /tmp/docker/images with all the tar archives in there.

my 2p is to make the GH workflow do this, as it is distracting in the normal code. In my go code, I only pull if the image doesn't already exist (or it is a locally built one). So, if you can have in the GH action the images safely restored from tar, then "pull if not exists" here for the non-test/tagged images will be simpler code. Also, it centralizes the responsibility vs picking a magic tar directory. When I look at other code that uses k3s or testcontainers it is a lot simpler than the code here, and I think we need to keep in mind how difficult this is vs the intent of testcontainers (just use it sort of thing)

Let me see if I got your point correct. You are suggesting to do the "docker load" part in the GH action itself, right? If so, good point, I did not think about it this way

yeah in fact when I have something complicated for GH action, I put it into a shell script and have the GH action call that. This way it is easier to ad-hoc test. I have it on my TODO to look at https://dagger.io/ instead of this approach

I took a closer look, and I don't think that your suggestion is possible. At this point, there is no instance of k3s starter, so no way to import an image via ctr. I think for such a thing to happen, we would need to re-think the way we do it.

I am really interested how you are doing it to be honest and where I can take a look.

And sorry it took a while to respond, I had to recollect why it is like this, and create that PR. Your comments triggered that, so thank you :)

thanks for raising it. Most of the time testcontainers-java is ahead of go, and this is a rare place where we need to catch up the main one. 👍

neah, thank you actually. I never saw the problem from this angle until you came here, so all the credits go to you. kudos!

Just to be clear when we run the tests locally (or internally on Jenkins) we will still pull the images like we did before because this cache will never exist?

that's correct. it's like running the test locally, without having the images already pulled

a new stage that basically does this:

• if a cache exists with key docker-images-cache-${{ hashFiles('**/current-images.txt') }} use it and restore to /tmp/docker/images
• if such a cache does not exist, read what is in current-images.txt line by line. For each of those lines, do a docker pull then a docker save to /tmp/docker/images
• if the above step happened, save to cache the directory /tmp/docker/images

load from either /tmp/docker/images (if it runs on github) or do a docker pull/docker save/docker load, just like we used to do until now